Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose inlay hints to Razor cohosting #74548

Merged
merged 2 commits into from
Jul 29, 2024

Conversation

davidwengier
Copy link
Contributor

Part of dotnet/razor#9519

Also exposes a test helper for better Razor tests

@davidwengier davidwengier requested a review from a team as a code owner July 25, 2024 06:12
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Jul 25, 2024
@davidwengier davidwengier requested a review from a team July 25, 2024 06:13
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

signing off. but DavidB should def look :)

@davidwengier
Copy link
Contributor Author

signing off. but DavidB should def look :)

Thank you Cyrus. @dibarbet you have been summoned :)

@davidwengier davidwengier merged commit 14e6d9a into dotnet:main Jul 29, 2024
25 checks passed
@davidwengier davidwengier deleted the RazorCohostInlayHints branch July 29, 2024 09:18
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Jul 29, 2024
@RikkiGibson RikkiGibson modified the milestones: Next, 17.12 P1 Jul 29, 2024
davidwengier added a commit to dotnet/razor that referenced this pull request Jul 31, 2024
Part of #9519
Needs dotnet/roslyn#74548 before it will compile
Needs
https://devdiv.visualstudio.com/DevDiv/_git/VSLanguageServerClient/pullrequest/567229
before it will work in VS

There were a few side quests on this one:
* Roslyn OOP, at least the way we access it, doesn't have any options
set, so took a few tries to get the Roslyn side of this right for our
needs
* I wrote this feature test-first so only discovered the lack of options
after I modified Roslyn and our test infra to allow us to set global
options, so ended up removing most of that code, Kept the bit about
isolated workspaces because it just makes sense.
* Had to re-write one of the `DocumentMappingService` methods to use
`TextChange` instead of `TextEdit` so I could use Roslyn LSP types
* The hacky, and fortunately temporary, way we were doing generated C#
content was causing cache misses in Roslyn in tests, so fixed that up
* When I finally got up to manual testing I found a bug in the platform
that meant inlay hints just don't work with dynamic registration, so
filed the above linked PR to fix that

If reviewing commit-at-a-time please note that the first commit was
written before the reworking of extension methods and LSP types, and the
fixes for that are in the last commit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants